Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Custom Client Directives #583

Merged
merged 5 commits into from
Jun 2, 2023
Merged

Custom Client Directives #583

merged 5 commits into from
Jun 2, 2023

Conversation

bluwy
Copy link
Member

@bluwy bluwy commented May 12, 2023

Summary

Support custom client: directives added by integrations

Links

@bluwy
Copy link
Member Author

bluwy commented May 13, 2023

I added a new non-goal, which I thought could be a common misconception later:

Non-goal:

  • Replay interaction on hydrate, e.g. if a client:click directive hydrates on clicking the button, Astro doesn't replay the click event to trigger some reaction after it hydrates. The user has to click the (now hydrated) button again to trigger a reaction.

Making this work ootb would be really tricky and I think it's better to leave the directives to handle this themselves if needed, since they should have a reference to the island element and event.target to do so.

@ematipico
Copy link
Member

ematipico commented May 16, 2023

Should we have a helper function called defineDirective to make the types more ergonomic?

const directive = defineDirective((load, opts, el)) => {})

@bluwy
Copy link
Member Author

bluwy commented May 16, 2023

I've thought about it before, but then we'd have to export a new entrypoint (astro/directive) and it's more tooling author-facing that I skipped it for now 😬 I guess a similar example is the image service API which could perhaps benefit with this pattern too.

But if we're in favour of it, I won't mind supporting that though.

@ematipico
Copy link
Member

I've thought about it before, but then we'd have to export a new entrypoint (astro/directive) and it's more tooling author-facing that I skipped it for now 😬 I guess a similar example is the image service API which could perhaps benefit with this pattern too.

You raise a good point. We can leave my suggestion for now, and think about it later!

Maybe we could merge all these define* functions inside a single astro/define entry point. Just a suggestion

@matthewp
Copy link
Contributor

I'm moving for a call for consensus on this RFC. Custom client directives has been in experimental since 2.5 without incident. No issues have been filed thus far and no feedback on the API. This will be the final comment period (3 days); if there are no objections this will be merged and the feature can move out of experimental in a future release (likely 2.6).

@matthewp
Copy link
Contributor

matthewp commented Jun 2, 2023

It's been 3 days, this RFC is merged!

@matthewp matthewp merged commit 7c91654 into main Jun 2, 2023
@matthewp matthewp deleted the custom-client-directives branch June 2, 2023 11:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants